feat(v4): HAD post-fit aggregate() + mode inference; per-level bootstrap-gate convergence (2(b) PR-4, M-027/M-139) - #751
Conversation
…rap-gate convergence (2(b) PR-4, M-027/M-139)
fit(aggregate=) and did_had_pretest_workflow(aggregate=) deprecate via
the NOT_SUPPLIED shim. Unlike every prior adopter the param was a MODE
SELECTOR over mutually exclusive panel shapes, so the sentinel resolves
by panel-shape inference (shared _infer_aggregate_mode, running after
the column-alias reconciliation): two distinct periods -> the overall
single-period WAS estimator/battery, more -> the event-study ones. A
plain multi-period fit() goes error->works; supplied legacy values warn
then run the legacy routing unchanged (validation survives). Post-fit
aggregate() lands on both results classes as PURE VIEWS with per-class
supported subsets and NO kit (legacy pickles aggregate identically):
'simple' on overall fits (target = the WAS estimand label, n = n_obs
units, provenance-exact df), 'event_study' on event-study fits (the
_from_had passthrough, cband included; its n_kind corrected obs->units).
summary() renders the target column + neutral estimate heading whenever
the single distinct target is not 'att' (sized to the longest label;
fixes dCDH's shipped ATT-mislabel; uniform-att and att/acrt output
byte-stable). HAD ES containers stay outside honest/pretrends -
DEFERRED wording corrected: coefficients ARE F-1-anchored, only the
anchor ROW is omitted and no joint covariance exists.
Per-level bootstrap-gate convergence (supersedes the recorded uniform
fail-closed decision of M-021..M-023): CS/EDiD/Imputation/TwoStage
aggregate('simple') is a bit-exact relay of the stored overall quintet,
so it now dispatches before the bootstrap gate and stays available on
bootstrapped fits - with a NaN df column (no df governs percentile
inference) - while the recompute levels keep failing closed. Normative
per-level rule added to v4-design section 6.
Ledger: M-027 shimmed; NEW row M-139 (121 rows; matrix + naming-guard
amendments incl. the 'aggregate' token + CONSUMER/SURFACE allowlists);
M-092 qualifier + M-093 amendment extended to HAD; M-020..M-026 note
convergence. Tests: 30-test HAD contract block (inference equivalence
bit-identical per shape, degenerate-inference relays, post-filter
sentinel regression, workflow twins), the four bootstrap pins flipped
into relay+recompute pairs with survey arms, consumer rejection pins,
llms-full shim + no-teach guards, notebook prose drift guard. Sweep:
125 test-site kwarg drops/migrations, practitioner/guides/REGISTRY/
tutorials/troubleshooting/CHANGELOG (incl. four unreleased blanket-
policy bullets), re-derived had.py line anchors. Pickles byte-identical
pre/post (no retention; views are O(1) relays).
Overall assessmentExecutive summary
MethodologyP1 — HAD event-study results are mislabeled as ATTLocation: Impact: HAD estimates Concrete fix: Add optional estimand metadata to All other reviewed methodology changes conform to the Registry’s documented M-027/M-139 and per-level bootstrap policies. Code QualityNo additional findings. PerformanceNo findings. The HAD aggregations are pure views, and bootstrap recomputation gates occur before expensive work. MaintainabilityNo additional findings. Tech DebtP3 — Tracked HAD event-study limitationsImpact: Joint covariance remains unavailable, and survey fits lack per-row df provenance. Concrete fix: No action required in this PR. These are tracked in SecurityNo findings or apparent secrets. Documentation/TestsP2 — Tutorial attributes residual variance to fitted treatment signalLocation: Impact: In the stated linear DGP, Concrete fix: Attribute the observed differences to horizon-specific errors, conditional variance, boundary leverage, and survey influence-function clustering. Also replace “cover zero — no pre-trends” with “include zero — no detected pre-trend.” Path to Approval
|
…erpretation prose (P2) P1: the HAD event-study container rendered WAS/WAS_d_lower numbers under summary()'s hardcoded ATT heading with no estimand metadata anywhere on the surface. New optional 'estimand' provenance field (appended last, the M-092 amendment convention), populated by _from_had from target_parameter, used as the summary() column heading (dynamic width) + a metadata-line entry, and serialized by to_dict(). Every other producer keeps estimand=None and renders byte-identically. Regression tests: WAS + WAS_d_lower heading/metadata/serialization pins + a non-HAD ATT byte-stability pin. P2: tutorial 22 attributed larger post-launch per-horizon SEs to the fitted treatment signal (slope * D), which the local-linear boundary fit absorbs as conditional-mean structure in this linear DGP; reworded to the error side (horizon-specific noise and PSU x period shock draws, conditional variance, boundary leverage, survey IF clustering) and 'cover zero - no pre-trends' -> 'include zero - no detected pre-trend'.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessmentExecutive summary
MethodologyP1 — DataFrame export still mislabels HAD’s estimandLocation: Impact: Concrete fix: Add an All other reviewed methodology changes conform to the documented M-027/M-139 and per-level bootstrap policies. Code QualityNo additional findings. PerformanceNo findings. MaintainabilityNo additional findings. Tech DebtP3 — Tracked HAD event-study limitationsImpact: Joint covariance remains unavailable, and survey fits lack per-row df provenance. Concrete fix: No action required in this PR; tracked in SecurityNo findings or apparent secrets. Documentation/TestsNo additional findings. Tutorial 22 now correctly attributes SE differences to horizon-specific errors, boundary leverage, and survey influence-function clustering. Path to Approval
|
The detached to_dataframe() frame still exported a bare att column, so
WAS-family numbers became indistinguishable from ATTs once separated
from the container. The pinned schema gains a per-row 'estimand'
discriminator appended last ('att' for every ATT producer; the
WAS/WAS_d_lower label for HAD - the AggregationResult.target
precedent), emitted by to_dataframe and recorded as an M-092 pre-cut
amendment. Regression pins extended to the frame for both HAD labels
and a non-HAD ATT producer.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — the prior P1 is resolved; no unmitigated P0/P1 findings remain. Executive summary
MethodologyNo unmitigated findings. The HAD mode inference, pure aggregation views, unit counts, and bootstrap behavior conform to the documented Registry note at Code QualityNo findings. PerformanceNo findings. MaintainabilityNo findings. Tech DebtP3 — Tracked inference limitationsImpact: HAD lacks joint cross-horizon covariance and per-row survey df provenance; recomputed bootstrap aggregation levels remain unavailable. Concrete fix: No action required in this PR. These are tracked in SecurityNo findings or apparent secrets. Documentation/TestsThe previous estimand-label defect is fully addressed:
No additional findings. The test suite could not run because NumPy, pandas, SciPy, and pytest are unavailable in the review environment. |
…/ absent The new test_notebook_prose_has_no_malformed_mode_phrases opened the tutorial notebooks with bare relative paths, so every CI test leg failed with FileNotFoundError when pytest ran outside the repo root (the only failure - 9,485 passed). Resolve via Path(__file__).parents[1] and pytest.skip when docs/ is absent (the tests/_tutorial_drift loader convention for the isolated-install job).
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — the prior P1 remains resolved; no unmitigated P0/P1 findings. Executive summary
MethodologyNo unmitigated findings. Affected methods are HAD ( The implementation matches these contracts at Code QualityNo findings. No new inline inference anti-pattern or incomplete parameter propagation was found. PerformanceNo findings. HAD aggregation remains a pure view without additional retained estimator state. MaintainabilityNo findings. The shared mode-inference helper prevents estimator/workflow routing drift. Tech DebtP3 — Tracked inference limitationsImpact: HAD still lacks joint cross-horizon covariance; recomputed bootstrap aggregation levels remain unavailable; several event-study producers lack per-row df provenance. Concrete fix: No action required in this PR. These limitations are tracked in SecurityNo findings or apparent secrets introduced. Documentation/TestsNo findings. The prior estimand-label issue is covered by The latest notebook-guard path correction is sound at Runtime tests could not run because NumPy and related dependencies are unavailable. Write-free compilation of 18 changed Python files and JSON parsing of all four changed notebooks succeeded. |
1 participant
Summary
HeterogeneousAdoptionDiD.fit(aggregate=)anddid_had_pretest_workflow(aggregate=)deprecate via the sharedNOT_SUPPLIEDshim. The param was a MODE SELECTOR over mutually exclusive panel shapes, so the sentinel resolves by panel-shape inference (shared_infer_aggregate_mode, running after the column-alias reconciliation): two distinct periods select the overall single-period WAS estimator/pretest battery, more select the event-study ones. A plain multi-periodfit()goes error→works; supplied legacy values warn then run the legacy routing unchanged (the pre-existing value validation survives).aggregate()on both HAD results classes as PURE VIEWS with per-class supported subsets and NO retained kit (results unpickled from any release aggregate identically):aggregate('simple')on overall fits is a one-row bit-exact relay whosetargetcarries the estimand label (WAS/WAS_d_lower), withn = n_obscontributing units and provenance-exact df;aggregate('event_study')on event-study fits is abuild_event_study_surfacepassthrough (cband fields included). The_from_hadadapter'sn_kindis corrected"obs"→"units".AggregationResult.summary()estimand-heading widening: the target column + neutralestimateheading now also render when the single distinct target is not"att"(column sized to the longest label) - previously HAD's WAS and dCDH's estimand-labelled relays would render under a hard-codedATTheading; uniform-attand ContinuousDiD att/acrt output are byte-stable.aggregate()level of a bootstrapped fit. Theiraggregate('simple')is a bit-exact relay of the stored overall quintet (percentile se/p/CI beside the finitesafe_inferencet), so it now dispatches before the bootstrap gate and stays available - with a NaN df column, because no df governs percentile inference - while the recompute levels keep failing closed. This supersedes the uniform-conservatism decision recorded with M-021..M-023; the normative per-level rule is added to v4-design §6.compute_honest_did/compute_pretrends_power- DEFERRED, not by-design, with corrected wording: the coefficients ARE reference-normalized (each horizon differences against the F-1 anchor); only the anchor row is omitted from the container, and no joint cross-horizon covariance exists.test_v4_matrixrange/count amendments; naming-guardaggregatetoken +CONSUMER_ALLOWLIST/SURFACE_ALLOWLISTentries with migrate-first discipline); M-092 scope qualifier and M-093 amendment extended to HAD.test_v4_rename_mechanical's no-FutureWarning pins), practitioner/guides/REGISTRY/tutorials/troubleshooting migrations, four unreleased CHANGELOG bullets amended to the per-level policy, re-derivedhad.pyline anchors repo-wide.Methodology references (required if estimator / math changes)
Validation
tests/test_aggregate_contract.py(30-test HAD block: no-warn inference pins per shape, bit-identical plain≡supplied equivalence, degenerate-inference relays, post-filter sentinel regression, workflow twins, summary-heading pins incl. dCDH; the four bootstrap fail-closed pins flipped into relay+recompute pairs with survey+bootstrap arms),tests/test_event_study_consumers.py(HAD rejection pins),tests/test_guides.py(shim-line + no-teach guards),tests/test_t21_had_pretest_workflow_drift.py(notebook prose guard), plus migrations acrosstest_had.py,test_had_pretests.py,test_had_dual_knob_deprecation.py,test_v4_rename_mechanical.py, drift suites,test_v4_matrix.py,test_naming_guard.py,test_practitioner.py.Security / privacy